Replace hosted_app_home build mode with copy_files mode#6868
Replace hosted_app_home build mode with copy_files mode#6868alfonso-noriega wants to merge 1 commit into02-wire-build-config-into-extension-specsfrom
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Coverage report
Test suite run success3850 tests passing in 1491 suites. Report generated by 🧪jest coverage report action from f042eff |
98aebcd to
802c958
Compare
1c5c718 to
3ac919f
Compare
802c958 to
b311dcc
Compare
2b6f7ba to
68c2a9f
Compare
db26c65 to
ec9b8bf
Compare
68c2a9f to
8096aa6
Compare
ec9b8bf to
973438a
Compare
f6ae0a0 to
1b503e0
Compare
71f4c4d to
c12d17e
Compare
1b503e0 to
01cfc8b
Compare
da8c9c5 to
c168755
Compare
048e70a to
c25acc0
Compare
c168755 to
146db7f
Compare
146db7f to
e56e3c3
Compare
c25acc0 to
7e39fc5
Compare
e56e3c3 to
ed19082
Compare
0b9bfe1 to
33feefb
Compare
| interface BuildConfig { | ||
| mode: 'ui' | 'theme' | 'function' | 'tax_calculation' | 'copy_files' | 'none' | ||
| steps: ReadonlyArray<BuildStep> | ||
| } |
There was a problem hiding this comment.
BuildConfig is now a breaking change: mode: 'none' requires steps
Previously BuildConfig was a union where {mode: 'none'} did not include steps. This PR changes it to an interface with mandatory steps for all modes, including none. Existing specs or runtime payloads that omit steps for none will fail type-checking or force dummy steps: [] everywhere.
Evidence: helper defaults updated to {mode: 'none', steps: []} only fixes callers using helpers; other constructors/deserializers remain broken.
Impact: TS compile failures and downstream breakage; may block builds/releases; affects any extension spec using {mode:'none'} without steps.
|
🤖 Code Review · #projects-dev-ai for questions ✅ Complete - No issues 📋 History✅ 1 findings → ✅ 1 findings → ✅ No issues |
f042eff to
fc9bd7b
Compare
61f4d35 to
8b922b5
Compare
| }, | ||
| }, | ||
| ], | ||
| }, |
There was a problem hiding this comment.
New copy_files mode is not part of BuildConfig union
This spec now sets buildConfig.mode: 'copy_files' with steps, but BuildConfig (in specification.ts) does not include 'copy_files' nor a steps field anywhere. This is a hard incompatibility: either TypeScript should fail, or if it’s being coerced, the build pipeline likely won’t know how to execute this mode and may ignore it or crash when it expects lifecycles. That can lead to static assets not being copied in dev/deploy, breaking Hosted App Home behavior for all users of that extension type.
fc9bd7b to
7e042f5
Compare
38a9028 to
a5303fd
Compare
5d3d410 to
17e1126
Compare
a5303fd to
0945d47
Compare
17e1126 to
df02d70
Compare
c468483 to
067c371
Compare
df02d70 to
3f8f751
Compare
067c371 to
be84850
Compare
3f8f751 to
00a05af
Compare
be84850 to
dd1d204
Compare
d46f3ad to
71b56c8
Compare
6e25f7a to
c8f1092
Compare
71b56c8 to
8d09cab
Compare
c8f1092 to
785afb4
Compare
7b2f1e7 to
f038316
Compare
785afb4 to
a7efd5b
Compare
a7efd5b to
e90a1b3
Compare
f038316 to
33cbfea
Compare

WHY are these changes introduced?
Fixes https://github.com/shop/issues-admin-extensibility/issues/2240
WHAT is this pull request doing?
How to test your changes?
Post-release steps
Measuring impact
How do we know this change was effective? Please choose one:
Checklist